Support Iceberg positional deletes #5897

yingsu00 · 2023-07-28T02:15:35Z

This PR is the first cut to support reading Iceberg tables with positional delete files.
Design doc: #5977

TODO:

Delete file deserialization in HiveIcebergSplit. This will be done after the Prestissimo changes are in shape
File path matching when reading the positional delete files
Performance optimization to avoid extra conversion from row numbers to bitmap then to row numbers

Catches:

The velox_hive_iceberg_datasource would be better to be included in velox_hive_connector. But so far it's unclear to me how to do it properly. Suggestions are welcome.

netlify · 2023-07-28T02:15:41Z

✅ Deploy Preview for meta-velox canceled.

Name	Link
🔨 Latest commit	`2ab2816`
🔍 Latest deploy log	https://app.netlify.com/sites/meta-velox/deploys/64cb2c8519e1b900086e4bcf

This is the first cut to support reading Iceberg tables with delete files. The design doc can be found at facebookincubator#5977

majetideepak

@yingsu00 Some high-level comments while we converge on the overall design.

majetideepak · 2023-08-04T16:28:24Z

velox/connectors/hive/iceberg/IcebergDataSource.cpp

+  auto& deleteFiles = icebergSplit->deleteFiles;
+  for (auto& deleteFile : deleteFiles) {
+    if (deleteFile.content == FileContent::kEqualityDeletes) {
+      readEqualityDeletes(deleteFile);


Move equality deletes to another PR.

majetideepak · 2023-08-04T16:30:05Z

velox/connectors/hive/storage_adapters/gcs/examples/CMakeLists.txt

@@ -20,5 +20,6 @@ target_link_libraries(
  velox_gcs
  velox_core
  velox_hive_connector
+  velox_hive_iceberg_datasource


These CMakeLists.txt additions should be redundant since velox_hive_iceberg_datasource is added to velox_hive_connector.

@majetideepak It didn't work and would throw link errors. Do you know how to make velox_hive_connector contain the velox_hive_iceberg_datasource? I tried to add PUBLIC link lib to velox_hive_iceberg_datasource, but it also doesn't work.

majetideepak · 2023-08-04T16:30:25Z

velox/connectors/hive/iceberg/IcebergSplit.cpp

+          _length,
+          _partitionKeys,
+          _tableBucketNumber) {
+  // TODO: Deserialize _extraFileInfo to get deleteFiles;


finish TODO.

finish TODO.

The delete file serialization is not done on the Presto_cpp yet, so I plan to do this in future PR after we decide how to serialize the delete files.

What are the open questions related to delete file serialization ? It might be better to finalize that to make this PR seem complete.

majetideepak · 2023-08-04T16:30:39Z

velox/connectors/hive/iceberg/IcebergDataSource.cpp

+
+  firstRowInSplit_ = rowReader_->nextRowNumber();
+
+  // TODO: Deserialize the std::vector<IcebergDeleteFile> deleteFiles. For now


finish TODO

Yuhta

@yingsu00 We probably want to create some DeltaProcessor interface class like this and call it in different parts of HiveDataSource, instead of subclassing HiveDataSource, because the split is not guaranteed to be the same type during the life cycle of the data source. At end of the day, DeltaProcessor is the property of split, not of data source.

class DeltaProcessor {
  // Can delegate to a factory registry to further decouple different implementations.
  static std::unique_ptr<DeltaProcessor> create(const HiveConnectorSplit&);

  // Called in HiveDataSource::addSplit to prepare delta readers.
  void prepareSplit();

  void readNext(RowReader&, int64_t size, VectorPtr& output);

  // In case delta readers need to be aware of filters.
  void addDynamicFilter();

  // Flush the data in delta reader out (e.g., in case we split update into delete + append, the newly appended rows need to be flushed here).
  void finishSplit();
};

Let me know how do you think. I can create some skeleton about this class and implement it for basic case (no delta).

Yuhta · 2023-08-09T16:54:08Z

velox/connectors/hive/iceberg/IcebergSplit.h

+namespace facebook::velox::connector::hive::iceberg {
+
+struct HiveIcebergSplit : public connector::hive::HiveConnectorSplit {
+  std::vector<IcebergDeleteFile> deleteFiles;


Are you parsing the delete file information in presto worker? Why not just parse it inside HiveIcebergDataSource in Velox? This way you do not need a specific split type for iceberg, because HiveConnectorSplit should already have all the information you need.

Yuhta · 2023-08-09T16:55:32Z

velox/connectors/hive/iceberg/IcebergMetadataColumn.h

+      : id(_id), name(_name), type(_type), doc(_doc) {}
+};
+
+#define ICEBERG_DELETE_FILE_PATH_COLUMN()  \


Why macros?

Yuhta · 2023-08-09T17:01:29Z

velox/connectors/hive/iceberg/IcebergDataSource.cpp

+  VELOX_NYI();
+}
+
+void HiveIcebergDataSource::openDeletes(


We cannot load all delete information in memory. You need to keep the delta readers here (I wrap it in some local class like DeltaDeleteReader/DeltaUpdateReader to encapsulate the logic). You can also keep the data sources but keep readers directly will make future optimization easier (e.g. overlapping loading of base file with decoding of delete files).

@Yuhta I intended to do this in future PRs. Do we have to do it now?

This will have a rather big impact on how you structure the thing, so better to do it right now, otherwise most of what you write now need to be rewritten later.

yingsu00 · 2023-08-12T02:21:47Z

because the split is not guaranteed to be the same type during the life cycle of the data source

@Yuhta Thanks for brainstorming. Could you give me an example when this is going to happen?

Yuhta · 2023-08-15T20:17:36Z

@yingsu00 For now this is not switch between two different frameworks, but could happen that the first split is a normal split without deltas but the subsequent one is a split with deltas. So process it at per split level looks the right thing to me.

aditi-pandit · 2023-08-15T23:36:22Z

velox/dwio/common/tests/utils/BatchMaker.cpp

@@ -41,7 +41,7 @@ template <typename T>
 VectorPtr createScalar(
    size_t size,
    std::mt19937& gen,
-    std::function<T()> val,
+    std::function<T(size_t)> val,


@yingsu00 : BatchMaker is deprecated. Can you use VectorMaker instead ?

yingsu00 · 2023-08-18T07:15:46Z

@yingsu00 For now this is not switch between two different frameworks, but could happen that the first split is a normal split without deltas but the subsequent one is a split with deltas. So process it at per split level looks the right thing to me.

This is fine. But the splits for reading an Iceberg table will always come in with extraFileInfo["table_format"]="iceberg"

Yuhta · 2023-08-18T16:44:22Z

This is fine. But the splits for reading an Iceberg table will always come in with extraFileInfo["table_format"]="iceberg"

This is not guaranteed for future table formats

yingsu00 · 2023-08-18T21:19:39Z

This is fine. But the splits for reading an Iceberg table will always come in with extraFileInfo["table_format"]="iceberg"

This is not guaranteed for future table formats

But Iceberg splits will always come in with this, similar to Hudi. We can guarantee that and their datasource can be constructed accordingly.

Yuhta · 2023-08-18T21:22:19Z

This is fine. But the splits for reading an Iceberg table will always come in with extraFileInfo["table_format"]="iceberg"

This is not guaranteed for future table formats

But Iceberg splits will always come in with this, similar to Hudi. We can guarantee that and their datasource can be constructed accordingly.

@yingsu00 We are design the framework for all these table formats, so dispatch at split level is the right thing here

yingsu00 · 2023-08-18T21:26:01Z

This is fine. But the splits for reading an Iceberg table will always come in with extraFileInfo["table_format"]="iceberg"

This is not guaranteed for future table formats

But Iceberg splits will always come in with this, similar to Hudi. We can guarantee that and their datasource can be constructed accordingly.

@yingsu00 We are design the framework for all these table formats, so dispatch at split level is the right thing here

But you can't mix everything together. These table formats spec are very different to each other and they are quite complex. THere has to be some abstractions, layers and separation of concerns. And how was the decision made?

assignUser · 2023-11-07T02:03:00Z

Superseded by #7362

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 28, 2023

yingsu00 force-pushed the iceberg4 branch 13 times, most recently from ecba7d1 to 88037c6 Compare August 3, 2023 02:41

yingsu00 marked this pull request as ready for review August 3, 2023 04:24

yingsu00 requested a review from Yuhta August 3, 2023 04:24

yingsu00 added 2 commits August 2, 2023 21:26

Add support to create ordered vector in BatchMaker

286eaba

Iceberg positional deletes read support

2ab2816

This is the first cut to support reading Iceberg tables with delete files. The design doc can be found at facebookincubator#5977

yingsu00 force-pushed the iceberg4 branch from 88037c6 to 2ab2816 Compare August 3, 2023 04:26

yingsu00 requested review from aditi-pandit and majetideepak August 3, 2023 05:48

majetideepak reviewed Aug 4, 2023

View reviewed changes

Yuhta reviewed Aug 9, 2023

View reviewed changes

Yuhta mentioned this pull request Aug 10, 2023

Iceberg read support in Velox #5977

Open

aditi-pandit reviewed Aug 16, 2023

View reviewed changes

felipepessoto mentioned this pull request Sep 29, 2023

[Gluten-core][VL] Supports Delta Lake Read apache/incubator-gluten#2902

Closed

yingsu00 mentioned this pull request Nov 2, 2023

Introducing IcebergSplitReader #7362

Closed

assignUser closed this Nov 7, 2023

YannByron mentioned this pull request Dec 8, 2023

[SUPPORT][SPARK][NATIVE] make hudi integrate into gluten/velox apache/hudi#10252

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support Iceberg positional deletes #5897

Support Iceberg positional deletes #5897

yingsu00 commented Jul 28, 2023 •

edited

Loading

netlify bot commented Jul 28, 2023 •

edited

Loading

majetideepak left a comment

majetideepak Aug 4, 2023

majetideepak Aug 4, 2023

yingsu00 Aug 12, 2023

majetideepak Aug 4, 2023

yingsu00 Aug 12, 2023

aditi-pandit Aug 16, 2023

majetideepak Aug 4, 2023

Yuhta left a comment •

edited

Loading

Yuhta Aug 9, 2023

Yuhta Aug 9, 2023

Yuhta Aug 9, 2023

yingsu00 Aug 12, 2023

Yuhta Aug 15, 2023 •

edited

Loading

yingsu00 commented Aug 12, 2023

Yuhta commented Aug 15, 2023

aditi-pandit Aug 15, 2023

yingsu00 commented Aug 18, 2023

Yuhta commented Aug 18, 2023

yingsu00 commented Aug 18, 2023

Yuhta commented Aug 18, 2023

yingsu00 commented Aug 18, 2023

assignUser commented Nov 7, 2023


		firstRowInSplit_ = rowReader_->nextRowNumber();

		// TODO: Deserialize the std::vector<IcebergDeleteFile> deleteFiles. For now

Support Iceberg positional deletes #5897

Support Iceberg positional deletes #5897

Conversation

yingsu00 commented Jul 28, 2023 • edited Loading

netlify bot commented Jul 28, 2023 • edited Loading

✅ Deploy Preview for meta-velox canceled.

majetideepak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Yuhta left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Yuhta Aug 15, 2023 • edited Loading

Choose a reason for hiding this comment

yingsu00 commented Aug 12, 2023

Yuhta commented Aug 15, 2023

Choose a reason for hiding this comment

yingsu00 commented Aug 18, 2023

Yuhta commented Aug 18, 2023

yingsu00 commented Aug 18, 2023

Yuhta commented Aug 18, 2023

yingsu00 commented Aug 18, 2023

assignUser commented Nov 7, 2023

yingsu00 commented Jul 28, 2023 •

edited

Loading

netlify bot commented Jul 28, 2023 •

edited

Loading

Yuhta left a comment •

edited

Loading

Yuhta Aug 15, 2023 •

edited

Loading